Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Baylike Walls and Windows #5547

Merged
merged 58 commits into from
Jul 7, 2023

Conversation

Mazianni
Copy link
Contributor

@Mazianni Mazianni commented May 30, 2023

About The Pull Request

Largely a port of code from Daedalus Dock and icons from Bay/Nebula. Lots of things need to be touched with this.

Why It's Good For The Game

It just looks better, imho.

Changelog

🆑

/:cl:

@github-actions github-actions bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 30, 2023
@github-actions github-actions bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 1, 2023
@nik707
Copy link
Contributor

nik707 commented Jun 4, 2023

finally, i can quit

@Mazianni
Copy link
Contributor Author

Mazianni commented Jun 6, 2023

  • TODO - unfuck the various painter devices.

@Mazianni
Copy link
Contributor Author

Mazianni commented Jun 9, 2023

love how map changes turn my diffs into +10,000 line monstrosities.

ktoma36
ktoma36 previously approved these changes Jun 25, 2023
Copy link
Collaborator

@ktoma36 ktoma36 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good from what I can see, ping me on discord if you do any other changes or when you feel like the PR is good to merge

@@ -0,0 +1,3 @@
#define BITSHIFT_LEFT(X, N) (X << (N))
#define BITSHIFT_RIGHT(X, N) (X >> (N))
#define BITFLAG(X) BITSHIFT_LEFT(1, X)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need these defines?

bitshifts are a pretty fundamental thing
i don't think it should be behind a #define, especially when it's longer to type the define out in most cases.

code/game/machinery/air_alarm.dm Outdated Show resolved Hide resolved
code/game/machinery/camera/camera.dm Outdated Show resolved Hide resolved
//color = ""

/obj/machinery/door/airlock/security/armory/allowed(mob/user)
if(get_security_level() in list("green","blue"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

y' sure about this? shouldn't hos/whoever be able to get in? or is this the "public" door?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this code already exists. it just shows up as something new because i moved all the subtypes and their shit to a new file.

code/game/machinery/doors/firedoor.dm Outdated Show resolved Hide resolved
code/modules/mob/living/carbon/human/update_icons.dm Outdated Show resolved Hide resolved
code/modules/power/apc.dm Outdated Show resolved Hide resolved
code/modules/power/lighting/lighting.dm Outdated Show resolved Hide resolved
code/modules/power/lighting/lighting.dm Outdated Show resolved Hide resolved
code/modules/power/lighting/lighting.dm Outdated Show resolved Hide resolved
Comment on lines +498 to +524
switch(D)
if(NORTH)
dir = WEST
break
if(EAST)
dir = SOUTH
break
if(SOUTH)
dir = WEST
break
if(WEST)
dir = SOUTH
break
if(T.density)
switch(D)
if(NORTH)
dir = WEST
break
if(EAST)
dir = SOUTH
break
if(SOUTH)
dir = WEST
break
if(WEST)
dir = SOUTH
break
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dir setting code should be before the . =..() and instead set ndir, so the proc itself sets it

setting ndir will make this happen because args are just passed down the ..() instead of copied so by re-setting an arg, it's propagating down.

directly dir setting after ..() will break things that rely on this behavior

Copy link
Contributor

@silicons silicons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last nitpicks

code/game/machinery/doors/firedoor.dm Outdated Show resolved Hide resolved
code/game/machinery/doors/firedoor.dm Show resolved Hide resolved
code/modules/power/apc.dm Show resolved Hide resolved
code/game/objects/structures/low_wall.dm Outdated Show resolved Hide resolved
Comment on lines 124 to 129
/obj/structure/wall_frame/CanPass(atom/movable/mover, turf/target, height=0, air_group=0)
if(air_group || (height==0)) return 1
if(istype(mover,/obj/projectile))
return 1
if(istype(mover) && mover.check_pass_flags(ATOM_PASS_TABLE))
return 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

our canPass doesn't touch ZAS, check the root definition of CanPass and use that; don't worry about atmos pass, that's already handled by /obj/structure's defaults.

code/game/objects/structures/low_wall.dm Outdated Show resolved Hide resolved
code/game/objects/structures/crates_lockers/__closet.dm Outdated Show resolved Hide resolved
code/game/objects/structures/crates_lockers/__closet.dm Outdated Show resolved Hide resolved
code/game/objects/structures/crates_lockers/__closet.dm Outdated Show resolved Hide resolved
code/game/objects/structures/crates_lockers/__closet.dm Outdated Show resolved Hide resolved
@silicons silicons merged commit 8cc591c into Citadel-Station-13:master Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants